[EuiBasicTable][EuiInMemoryTable] Enable more action props to accept an optional callback + fix missing tooltips on collapsed actions #7373

cee-chen · 2023-11-17T02:26:05Z

Summary

@alvarezmelissa87 is working on a table that needs conditional action descriptions. She was previously attempting to use custom rendered actions to achieve this, but it would be much cleaner all around for their use case for EUI to simply allow actions.description to be a callback which we pass the current item to.

I also added this functionality to the href and data-test-subj keys while here, because why not/it seems useful! Example usage:

<EuiBasicTable
  columns={[
    // ... columns
   {
      name: 'Actions',
      actions: [{
        name: (item) => `Delete ${item.name}`,
        description: (item) => item.someCondition ? 'This does X' : 'This does Y',
        href: (item) => `/delete/${item.id}`,
        'data-test-subj': (item) => `delete-${item.id}`,
      }],
    },
  ]}
/>

Actions tooltip bugfix

While here, I noticed that the action descriptions (which normally renders as a delayed tooltip) was not correctly showing in the collapsed popover menu when they should be, so I added that in d29b623:

Doing this required adding a new toolTipsProps prop to EuiContextMenu so we could pass delay="long" to match the other actions tooltips (ba28dc0)

As always, please review by commit (because I found a few more bugs even more bugs after that 🙈)

QA

Go to https://eui.elastic.co/pr_7373/#/tabular-content/tables#adding-actions-to-table
Hover or focus on a non-disabled link action (online rows are enabled, offline rows are disabled)
Confirm the action description tooltip shows the row name
Toggle the "Multiple Actions" switch in the top left corner
Confirm that keyboard tabbing into the actions column correctly makes the action buttons visible, unlike production
When clicking the "All actions" button, confirm that actions in the popover menu have tooltips, unlike production

General checklist

Browser QA
~~- [ ] Checked in both light and dark modes~~
- [ ] Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
Docs site QA
- Updated documentation
- Props have proper autodocs (using @default if default values are missing) and playground toggles
  ~~- [ ] Checked Code Sandbox works for any docs examples~~
Code quality checklist
- Added or updated jest ~~and cypress tests~~
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist
~~- [ ] Updated the Figma library counterpart~~

…typeof check to a util

- convert tests to RTL, and remove unnecessary extra snapshot - remove TS comment, and replace extends with `,` workaround

+ `collapsed_item_actions` is repeating a util already in there unnecessarily, DRY it out too

+ silence `act()` errors caused by the fact that we load multiple versions of testing-library/DOM (tests still pass otherwise)

…actions - this involves adding `toolTipProps` to `EuiContextMenuItem` - deprecate a few other top-level tooltip props while here, for simpler API usage

+ update actions example to include props table

… to multiple children

- was broken for `<a>` links but not `<button>`s, doh

kibanamachine · 2023-11-17T05:40:34Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7373/
Storybook: https://eui.elastic.co/pr_7373/storybook

elasticmachine · 2023-11-17T05:40:38Z

💚 Build Succeeded

Buildkite Build
Commit: 32bce13

History

💚 Build #1075 succeeded 86af956
💔 Build #1073 failed f65cd36

1Copenut

👍 LGTM! I left a couple comments about Safari + VO not honoring tooltips anymore, but that appears to be a change / bug introduced by Apple.

1Copenut · 2023-11-20T16:53:27Z

src/components/basic_table/default_item_action.tsx

+    ariaLabelledBy = (
+      <EuiScreenReaderOnly>
+        <span id={ariaLabelId}>{actionContent}</span>
+      </EuiScreenReaderOnly>


This will go a long way to ensuring screen reader users have context when tooltips appear. Users will know exactly what their next keypress will do in terms of adding / editing / deleting.

VoiceOver on Safari has changed the way it announces and handles aria-describedby and I haven't found much to explain why. This combination doesn't announce the tooltip text at all, though VoiceOver + Firefox does. Windows screen readers announce the tooltips properly.

The only thing I could find was this issue ticket filed last year that references the WCAG samples not working correctly.

I noticed this as well when I was testing tooltips on Safari+VO! But I found the same behavior on the EuiToolTip page as well so I concluded it wasn't my changes causing it 😬 I do worry a bit that maybe something we did on EUI caused it, let me try to dig into it a bit more separately from this PR

`v91.0.0-backport.0`⏩`v91.3.1` ⚠️ The largest set of changes in this PR that touch source code (as opposed to test code) are related to several **EuiDataGrid** redesigns, particularly around the toolbar, column cell headers, and cell actions. We **strongly** recommend QAing your EuiDataGrid usages, **especially** if you have custom CSS styling on data grid cells. | Changes | Screencap | |--------|--------| | Cell actions and popover | <img src="https://github.com/elastic/kibana/assets/549407/6462d983-307f-4a3c-84b1-36d9b276c9a0" width="240" alt=""> | | Column headers | <img src="https://github.com/elastic/kibana/assets/549407/3fd64a15-829a-48f3-9dba-9dae3c73e6b2" alt="" width="360"> | | Toolbar | <img src="https://github.com/elastic/kibana/assets/549407/f876f6d7-635d-497a-b1e7-9daf4e6fd3e3" alt="" width="240"> | --- ## [`v91.3.1`](https://github.com/elastic/eui/releases/v91.3.1) **Bug fixes** - Moved `EuiDataGrid`'s header cells' `dataGridHeaderCellActionButton` test subject attribute from to the clickable button, for easier E2E testing ([#7427](elastic/eui#7427)) - Fixed `EuiBasicTable`/`EuiInMemoryTable` actions to correctly show as disabled when rows are being selected ([#7428](elastic/eui#7428)) ## [`v91.3.0`](https://github.com/elastic/eui/releases/v91.3.0) - Added `esqlVis`, `pipeBreaks`, and `pipeNoBreaks` icon glyphs. ([#7399](elastic/eui#7399)) - Updated `EuiDataGridSchemaDetector`'s comparator arguments to include entry indexes ([#7406](elastic/eui#7406)) ## [`v91.2.0`](https://github.com/elastic/eui/releases/v91.2.0) - Added `endpoint` glyph to `EuiIcon` ([#7383](elastic/eui#7383)) **Bug fixes** - Fixed a bug with `EuiSelectable`s with custom `truncationProps`, where scrollbar widths were not being accounted for ([#7392](elastic/eui#7392)) ## [`v91.1.0`](https://github.com/elastic/eui/releases/tag/v91.1.0) - Updated `EuiDataGrid` cell actions to display above cells instead of within them, to avoid content clipping issues ([#7343](elastic/eui#7343)) - Updated `EuiDataGrid` cell expansion popovers to sit on top of cells instead of below/next to them ([#7343](elastic/eui#7343)) - Updated `EuiListGroupItem` to render an external icon and screen reader affordance for links with `target` set to to `_blank` ([#7352](elastic/eui#7352)) - Updated `EuiListGroupItem` with a new `external` prop, which allows enabling or disabling the new external link icon ([#7352](elastic/eui#7352)) - Updated `EuiText` to no longer set any opinionated styles on child `<img>` tags - use `EuiImage` for image display within text instead ([#7360](elastic/eui#7360)) - Improved `EuiBasicTable`/`EuiInMemoryTable`s mobile UI for custom actions ([#7361](elastic/eui#7361)) - Added a new `EuiDataGridToolbarControl` subcomponent, which is useful for rendering your own custom `EuiDataGrid` toolbar buttons while matching the look of the default controls ([#7369](elastic/eui#7369)) - Updated `EuiDataGrid`'s toolbar controls to show active/current counts in badges, and updated the Columns button icon ([#7369](elastic/eui#7369)) - Updated `EuiButtonEmpty` to allow passing `false` to `textProps`, which allows rendering custom button content without an extra text wrapper ([#7369](elastic/eui#7369)) - Updated `EuiDataGrid` column header cells to show the sort arrow after the heading text, instead of before ([#7371](elastic/eui#7371)) - Updated `EuiDataGrid`'s column header actions icon from a chevron to `boxesVertical` ([#7371](elastic/eui#7371)) - Updated the actions column in `EuiBasicTable` and `EuiInMemoryTable`s. Alongside `name`, the `description`, `href`, and `data-test-subj` properties now also accept an optional callback that the current `item` will be passed to ([#7373](elastic/eui#7373)) - Updated `EuiContextMenuItem` with a new `toolTipProps` prop ([#7373](elastic/eui#7373)) - `EuiSelectable` now allows configurable text truncation via `listProps.truncationProps` ([#7388](elastic/eui#7388)) - `EuiTextTruncate` now supports a new `calculationDelayMs` prop for working around font loading or layout shifting scenarios ([#7388](elastic/eui#7388)) **Bug fixes** - Fixed incorrect `EuiPopover` positioning calculations when `hasArrow` was set to false ([#7343](elastic/eui#7343)) - Fixed `EuiSuperSelect` to render options with falsy values (false, 0, and ''), but not nullish values (undefined or null) ([#7362](elastic/eui#7362)) - Fixed `EuiSuperSelect`'s typing to allow non-string values (e.g., booleans or numbers) ([#7362](elastic/eui#7362)) - Fixed `EuiDataGrid`'s numeric and currency column heading cells to be correctly right-aligned ([#7371](elastic/eui#7371)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not showing tooltip descriptions when rendered in the all actions popover menu ([#7373](elastic/eui#7373)) - Fixed missing underlines on `EuiContextMenu` link hover ([#7373](elastic/eui#7373)) - Fixed visual text truncation of `EuiBreadcrumb`s with `popoverContent` ([#7375](elastic/eui#7375)) - Fixed `EuiFormRow`s with `hasEmptyLabelSpace` being very slightly off in vertical alignment ([#7380](elastic/eui#7380)) **Deprecations** - Deprecated `EuiContextMenuItem`'s `toolTipTitle` prop. Use `toolTipProps.title` instead ([#7373](elastic/eui#7373)) - Deprecated `EuiContextMenuItem`'s `toolTipPosition` prop. Use `toolTipProps.position` instead ([#7373](elastic/eui#7373)) **Accessibility** - Fixed custom `EuiBasicTable`/`EuiInMemoryTable` rendering nested interactive custom actions ([#7361](elastic/eui#7361)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not correctly reading out action descriptions to screen readers ([#7373](elastic/eui#7373)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` primary actions not visibly appearing on keyboard focus ([#7373](elastic/eui#7373)) --------- Co-authored-by: Julia Rechkunova <[email protected]>

`v91.0.0-backport.0`⏩`v91.3.1` ⚠️ The largest set of changes in this PR that touch source code (as opposed to test code) are related to several **EuiDataGrid** redesigns, particularly around the toolbar, column cell headers, and cell actions. We **strongly** recommend QAing your EuiDataGrid usages, **especially** if you have custom CSS styling on data grid cells. | Changes | Screencap | |--------|--------| | Cell actions and popover | <img src="https://github.com/elastic/kibana/assets/549407/6462d983-307f-4a3c-84b1-36d9b276c9a0" width="240" alt=""> | | Column headers | <img src="https://github.com/elastic/kibana/assets/549407/3fd64a15-829a-48f3-9dba-9dae3c73e6b2" alt="" width="360"> | | Toolbar | <img src="https://github.com/elastic/kibana/assets/549407/f876f6d7-635d-497a-b1e7-9daf4e6fd3e3" alt="" width="240"> | --- ## [`v91.3.1`](https://github.com/elastic/eui/releases/v91.3.1) **Bug fixes** - Moved `EuiDataGrid`'s header cells' `dataGridHeaderCellActionButton` test subject attribute from to the clickable button, for easier E2E testing ([elastic#7427](elastic/eui#7427)) - Fixed `EuiBasicTable`/`EuiInMemoryTable` actions to correctly show as disabled when rows are being selected ([elastic#7428](elastic/eui#7428)) ## [`v91.3.0`](https://github.com/elastic/eui/releases/v91.3.0) - Added `esqlVis`, `pipeBreaks`, and `pipeNoBreaks` icon glyphs. ([elastic#7399](elastic/eui#7399)) - Updated `EuiDataGridSchemaDetector`'s comparator arguments to include entry indexes ([elastic#7406](elastic/eui#7406)) ## [`v91.2.0`](https://github.com/elastic/eui/releases/v91.2.0) - Added `endpoint` glyph to `EuiIcon` ([elastic#7383](elastic/eui#7383)) **Bug fixes** - Fixed a bug with `EuiSelectable`s with custom `truncationProps`, where scrollbar widths were not being accounted for ([elastic#7392](elastic/eui#7392)) ## [`v91.1.0`](https://github.com/elastic/eui/releases/tag/v91.1.0) - Updated `EuiDataGrid` cell actions to display above cells instead of within them, to avoid content clipping issues ([elastic#7343](elastic/eui#7343)) - Updated `EuiDataGrid` cell expansion popovers to sit on top of cells instead of below/next to them ([elastic#7343](elastic/eui#7343)) - Updated `EuiListGroupItem` to render an external icon and screen reader affordance for links with `target` set to to `_blank` ([elastic#7352](elastic/eui#7352)) - Updated `EuiListGroupItem` with a new `external` prop, which allows enabling or disabling the new external link icon ([elastic#7352](elastic/eui#7352)) - Updated `EuiText` to no longer set any opinionated styles on child `<img>` tags - use `EuiImage` for image display within text instead ([elastic#7360](elastic/eui#7360)) - Improved `EuiBasicTable`/`EuiInMemoryTable`s mobile UI for custom actions ([elastic#7361](elastic/eui#7361)) - Added a new `EuiDataGridToolbarControl` subcomponent, which is useful for rendering your own custom `EuiDataGrid` toolbar buttons while matching the look of the default controls ([elastic#7369](elastic/eui#7369)) - Updated `EuiDataGrid`'s toolbar controls to show active/current counts in badges, and updated the Columns button icon ([elastic#7369](elastic/eui#7369)) - Updated `EuiButtonEmpty` to allow passing `false` to `textProps`, which allows rendering custom button content without an extra text wrapper ([elastic#7369](elastic/eui#7369)) - Updated `EuiDataGrid` column header cells to show the sort arrow after the heading text, instead of before ([elastic#7371](elastic/eui#7371)) - Updated `EuiDataGrid`'s column header actions icon from a chevron to `boxesVertical` ([elastic#7371](elastic/eui#7371)) - Updated the actions column in `EuiBasicTable` and `EuiInMemoryTable`s. Alongside `name`, the `description`, `href`, and `data-test-subj` properties now also accept an optional callback that the current `item` will be passed to ([elastic#7373](elastic/eui#7373)) - Updated `EuiContextMenuItem` with a new `toolTipProps` prop ([elastic#7373](elastic/eui#7373)) - `EuiSelectable` now allows configurable text truncation via `listProps.truncationProps` ([elastic#7388](elastic/eui#7388)) - `EuiTextTruncate` now supports a new `calculationDelayMs` prop for working around font loading or layout shifting scenarios ([elastic#7388](elastic/eui#7388)) **Bug fixes** - Fixed incorrect `EuiPopover` positioning calculations when `hasArrow` was set to false ([elastic#7343](elastic/eui#7343)) - Fixed `EuiSuperSelect` to render options with falsy values (false, 0, and ''), but not nullish values (undefined or null) ([elastic#7362](elastic/eui#7362)) - Fixed `EuiSuperSelect`'s typing to allow non-string values (e.g., booleans or numbers) ([elastic#7362](elastic/eui#7362)) - Fixed `EuiDataGrid`'s numeric and currency column heading cells to be correctly right-aligned ([elastic#7371](elastic/eui#7371)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not showing tooltip descriptions when rendered in the all actions popover menu ([elastic#7373](elastic/eui#7373)) - Fixed missing underlines on `EuiContextMenu` link hover ([elastic#7373](elastic/eui#7373)) - Fixed visual text truncation of `EuiBreadcrumb`s with `popoverContent` ([elastic#7375](elastic/eui#7375)) - Fixed `EuiFormRow`s with `hasEmptyLabelSpace` being very slightly off in vertical alignment ([elastic#7380](elastic/eui#7380)) **Deprecations** - Deprecated `EuiContextMenuItem`'s `toolTipTitle` prop. Use `toolTipProps.title` instead ([elastic#7373](elastic/eui#7373)) - Deprecated `EuiContextMenuItem`'s `toolTipPosition` prop. Use `toolTipProps.position` instead ([elastic#7373](elastic/eui#7373)) **Accessibility** - Fixed custom `EuiBasicTable`/`EuiInMemoryTable` rendering nested interactive custom actions ([elastic#7361](elastic/eui#7361)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` actions not correctly reading out action descriptions to screen readers ([elastic#7373](elastic/eui#7373)) - Fixed `EuiBasicTable` and `EuiInMemoryTable` primary actions not visibly appearing on keyboard focus ([elastic#7373](elastic/eui#7373)) --------- Co-authored-by: Julia Rechkunova <[email protected]>

cee-chen added 5 commits November 16, 2023 19:04

Allow description to be a callback that changes depending on item

792b31b

Let other props that seem useful take callbacks as well, and DRY out …

bf1aaec

…typeof check to a util

[tech debt] misc DefaultItemAction cleanup

b038a8c

- convert tests to RTL, and remove unnecessary extra snapshot - remove TS comment, and replace extends with `,` workaround

[setup] DRY out utils to action_types

6cf4211

+ `collapsed_item_actions` is repeating a util already in there unnecessarily, DRY it out too

Update default actions in collapsed popover

26b2a79

cee-chen force-pushed the table-actions-continued branch 3 times, most recently from 17c0dd7 to fae65fa Compare November 17, 2023 04:58

cee-chen added 6 commits November 16, 2023 21:05

Fix collapsed popovers not displaying descriptions as tooltips

af61c9b

+ silence `act()` errors caused by the fact that we load multiple versions of testing-library/DOM (tests still pass otherwise)

Fix collapsed action to have the same tooltip delay as non-collapsed …

57e6003

…actions - this involves adding `toolTipProps` to `EuiContextMenuItem` - deprecate a few other top-level tooltip props while here, for simpler API usage

[docs] Add examples of callback props

9768b5a

+ update actions example to include props table

Fix tooltip aria-describedby ID not correctly applying to buttons due…

87abdd3

… to multiple children

Fix primary actions not correctly becoming visible on keyboard focus

f3da152

[EuiContextMenu] Fixed missing underlines on hover

32bce13

- was broken for `<a>` links but not `<button>`s, doh

cee-chen force-pushed the table-actions-continued branch from fae65fa to 32bce13 Compare November 17, 2023 05:06

cee-chen marked this pull request as ready for review November 17, 2023 05:07

cee-chen requested a review from a team as a code owner November 17, 2023 05:07

1Copenut approved these changes Nov 20, 2023

View reviewed changes

cee-chen merged commit ada8592 into elastic:main Nov 20, 2023
8 checks passed

cee-chen deleted the table-actions-continued branch November 20, 2023 21:34

cee-chen mentioned this pull request Nov 20, 2023

[VoiceOver+Safari] Tooltips / aria-describedby on interactive elements are no longer read out by VO #7377

Closed

cee-chen mentioned this pull request Nov 29, 2023

[Meta] Deprecation schedule #1469

Open

35 tasks

This was referenced Dec 18, 2023

Upgrade EUI to v91.3.1 elastic/kibana#173569

Merged

[EuiBasicTable] Fix regressed disabled actions UX when selecting rows #7428

Merged

cee-chen mentioned this pull request Jan 2, 2024

[EuiContextMenu] Missing underline hover on link items in Firefox #7429

Closed

cee-chen mentioned this pull request Jan 22, 2024

[EuiContextMenuItem] Fix underline styling on hover for non-links #7474

Merged

4 tasks

cee-chen mentioned this pull request Jan 26, 2024

[EuIContextMenuItem] Remove deprecated toolTipTitle and toolTipPosition props #7489

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiBasicTable][EuiInMemoryTable] Enable more action props to accept an optional callback + fix missing tooltips on collapsed actions #7373

[EuiBasicTable][EuiInMemoryTable] Enable more action props to accept an optional callback + fix missing tooltips on collapsed actions #7373

cee-chen commented Nov 17, 2023 •

edited by 1Copenut

Loading

kibanamachine commented Nov 17, 2023

elasticmachine commented Nov 17, 2023

1Copenut left a comment

1Copenut Nov 20, 2023

1Copenut Nov 20, 2023

cee-chen Nov 20, 2023

[EuiBasicTable][EuiInMemoryTable] Enable more action props to accept an optional callback + fix missing tooltips on collapsed actions #7373

[EuiBasicTable][EuiInMemoryTable] Enable more action props to accept an optional callback + fix missing tooltips on collapsed actions #7373

Conversation

cee-chen commented Nov 17, 2023 • edited by 1Copenut Loading

Summary

Actions tooltip bugfix

QA

General checklist

kibanamachine commented Nov 17, 2023

elasticmachine commented Nov 17, 2023

💚 Build Succeeded

History

1Copenut left a comment

Choose a reason for hiding this comment

1Copenut Nov 20, 2023

Choose a reason for hiding this comment

1Copenut Nov 20, 2023

Choose a reason for hiding this comment

cee-chen Nov 20, 2023

Choose a reason for hiding this comment

cee-chen commented Nov 17, 2023 •

edited by 1Copenut

Loading